Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add clipping treatment for negative values of non-negative tracers #682

Merged
merged 3 commits into from
Jan 24, 2024
Merged

add clipping treatment for negative values of non-negative tracers #682

merged 3 commits into from
Jan 24, 2024

Conversation

AmandaBack-NOAA
Copy link
Contributor

@AmandaBack-NOAA AmandaBack-NOAA commented Jan 11, 2024 •
edited by RussTreadon-NOAA
Loading

Description

Clips negative values from fields in the tracer file before writing. Fixes #680.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Tested on Hera in a RRFS CONUS workflow with both hybrid_radar_ref and anal_gsi_input tasks.
See run directory at /scratch2/BMC/zrtrr/aback/hydrometeors_test/
For one hour of the control run without this change, and one hour using the change contained in this PR, the INPUT directory was saved for reference/rerunning before each of the tasks: anal_gsi_input_spinup, hybrid_radar_ref_spinup, cldanl_nonvar_spinup, and run_fcst spinup. Those directories are at, for example /scratch2/BMC/zrtrr/aback/hydrometeors_test/INPUT_after_anal_gsi_input_spinup_before_hybrid_radar_ref_spinup ("control") and /scratch2/BMC/zrtrr/aback/hydrometeors_test/clipped_INPUT_after_anal_gsi_input_spinup_before_hybrid_radar_ref_spinup
(using the code in the PR).
Logs are at /scratch2/BMC/zrtrr/aback/hydrometeors_test/v0.7.9/logs/rrfs.20230610/08/, including new logs produced between all the above-listed tasks, showing all the min and max values in fv_tracer.

Checklist

The code is Ting Lei's. It looks stylish and understandable to me (Amanda) but maybe Ting should weigh in on this section.

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing tests pass with my changes
  • Any dependent changes have been merged and published

DUE DATE for merger of this PR into develop is 2/22/2024 (six weeks after PR creation).

hongli-wang
hongli-wang approved these changes Jan 12, 2024
View reviewed changes
Copy link
Collaborator

@hongli-wang hongli-wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me. I approve this PR.

@ShunLiu-NOAA
Copy link
Contributor

@AmandaBack-NOAA Could you run regression test on Hera, Orion and WCOSS2? Thanks.

@ShunLiu-NOAA ShunLiu-NOAA self-requested a review January 17, 2024 16:34
@hu5970
Copy link
Collaborator

hu5970 commented Jan 19, 2024

@AmandaBack-NOAA @TingLei-NOAA Could you sync with current develop branch? Thanks, Ming

@AmandaBack-NOAA
Copy link
Contributor Author

@hu5970 synced.

@hu5970
Copy link
Collaborator

hu5970 commented Jan 19, 2024

@AmandaBack-NOAA Thanks.

@hu5970
Copy link
Collaborator

hu5970 commented Jan 22, 2024

The regression test results on Hera:

[Ming.Hu@hfe03 build]$ ctest -j7
Test project /scratch1/BMC/wrfruc/mhu/code/gsi/GSI/build
    Start 1: global_4denvar
    Start 2: rtma
    Start 3: rrfs_3denvar_glbens
    Start 4: netcdf_fv3_regional
    Start 5: hafs_4denvar_glbens
    Start 6: hafs_3denvar_hybens
    Start 7: global_enkf
1/7 Test #4: netcdf_fv3_regional ..............   Passed  610.69 sec
2/7 Test #3: rrfs_3denvar_glbens ..............***Failed  619.89 sec
3/7 Test #7: global_enkf ......................   Passed  1083.49 sec
4/7 Test #2: rtma .............................   Passed  1155.76 sec
5/7 Test #6: hafs_3denvar_hybens ..............***Failed  1347.53 sec
6/7 Test #5: hafs_4denvar_glbens ..............***Failed  1412.45 sec
7/7 Test #1: global_4denvar ...................   Passed  1670.17 sec

57% tests passed, 3 tests failed out of 7

Total Test time (real) = 1670.22 sec

The following tests FAILED:
          3 - rrfs_3denvar_glbens (Failed)
          5 - hafs_4denvar_glbens (Failed)
          6 - hafs_3denvar_hybens (Failed)
Errors while running CTest
Output from these tests are in: /scratch1/BMC/wrfruc/mhu/code/gsi/GSI/build/Testing/Temporary/LastTest.log
Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.

The reason of failure for three regional cases is:

The fv3_dynvars are reproducible
The fv3_sfcdata are reproducible
The results between the two runs (rrfs_3denvar_glbens_loproc_updat and rrfs_3denvar_glbens_loproc_contrl) are not reproducible
Thus, the case has Failed siganl of the regression tests.

It is the difference of the fv3_tracer files between control and update runs caused failure. When diff those two tracer files with ncdiff:
ncdiff hafs_4denvar_glbens_loproc_updat/fv3_tracer hafs_4denvar_glbens_loproc_contrl/fv3_tracer diff_tracer.nc
Only sphum fields show positive values. All other fields are identical.

The same diff results are also true for "hafs_3denvar_hybens" and "rrfs_3denvar_glbens".

This PR is to "Clips negative values from fields in the tracer file before writing". This difference in moisture field is expected.
So the regression tests are passed.

@hu5970
Copy link
Collaborator

hu5970 commented Jan 22, 2024 •
edited
Loading

The regression test results from Cactus:

[ming.hu@clogin05 build] ctest -j7
Test project /lfs/h2/emc/ptmp/Ming.Hu/gsi/GSI/build
    Start 1: global_4denvar
    Start 2: rtma
    Start 3: rrfs_3denvar_glbens
    Start 4: netcdf_fv3_regional
    Start 5: hafs_4denvar_glbens
    Start 6: hafs_3denvar_hybens
    Start 7: global_enkf
1/7 Test #4: netcdf_fv3_regional ..............   Passed  483.52 sec
2/7 Test #7: global_enkf ......................   Passed  621.34 sec
3/7 Test #3: rrfs_3denvar_glbens ..............***Failed  665.50 sec
4/7 Test #6: hafs_3denvar_hybens ..............***Failed  1213.37 sec
5/7 Test #5: hafs_4denvar_glbens ..............***Failed  1284.06 sec
6/7 Test #1: global_4denvar ...................   Passed  1567.19 sec

Test project /lfs/h2/emc/ptmp/Ming.Hu/gsi/GSI/build
    Start 2: rtma
1/1 Test #2: rtma .............................   Passed  968.63 sec

100% tests passed, 0 tests failed out of 1

Total Test time (real) = 968.63 sec

The reason of failure for three regional cases are the same as the results from Hera tests:

The fv3_dynvars are reproducible
The fv3_sfcdata are reproducible
The results between the two runs (rrfs_3denvar_glbens_loproc_updat and rrfs_3denvar_glbens_loproc_contrl) are not reproducible
Thus, the case has Failed siganl of the regression tests.

So the regression test on Wcoss2 passed.

@TingLei-NOAA
Copy link
Contributor

TingLei-NOAA commented Jan 22, 2024 via email

@ShunLiu-NOAA ShunLiu-NOAA merged commit 12f8d3e into NOAA-EMC:develop Jan 24, 2024
@RussTreadon-NOAA
Copy link
Contributor

While this PR is closed, I was curious as to why update_guess.f90 was not effective in ensuring a non-negative sphum analysis increment. The rrfs_3denvar_glbens ctest was used to investigate negative sphum values.

Print statements were added to update_guess.f90 and gsi_rfv3io_mod.f90.

  • confirm that update_guess ensures sphum analysis is greater or equal to the specified bound qcmin which is set to 1.0e-7 in constants.f90. That is, sphum is positive definite on the GSI analysis grid.
  • prints added to gsi_rfv3io_mod.f90 for sphum show that adding workb2 to work_b can result in negative values in work_b. Logic added by this PR detects this and resets negative work_b to 0.0

Is it possible that that negative values of sphum originate as a result of operations performed in fv3_h_to_ll?

@hongli-wang
Copy link
Collaborator

That is a good question. Ting and I had discussed about this. fv3_h_to_ll could be one of possible reasons. Ting had other finding and he may add more about it.

@TingLei-NOAA
Copy link
Contributor

I have no more to add to @hongli-wang and yes, we think that fv3_ll_to_h is the reason for negative values in q/sphum. Thanks to @RussTreadon-NOAA 's digging in and verification of its actual behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TingLei-NOAA TingLei-NOAA TingLei-NOAA approved these changes

@RussTreadon-NOAA RussTreadon-NOAA RussTreadon-NOAA left review comments

@ShunLiu-NOAA ShunLiu-NOAA ShunLiu-NOAA approved these changes

@hongli-wang hongli-wang hongli-wang approved these changes

@hu5970 hu5970 Awaiting requested review from hu5970

Assignees

@ShunLiu-NOAA ShunLiu-NOAA

@AmandaBack-NOAA AmandaBack-NOAA

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Negative sphum and hydrometeors from RRFS GSI
6 participants
@AmandaBack-NOAA @ShunLiu-NOAA @hu5970 @TingLei-NOAA @RussTreadon-NOAA @hongli-wang